Dev -> Main#96
Conversation
Client/header
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try { | ||
| const biomePath = join(process.cwd(), "biome.json") | ||
| await execAsync( | ||
| `yarn biome check --write --unsafe --config-path=${biomePath} ${shadowPath}`, |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, dynamically constructed shell commands that include environment- or filesystem-derived values should avoid passing a single interpolated string to a shell. Instead, call the executable directly and pass arguments in an array using APIs like execFile/spawn (or execFileSync/spawnSync), so the shell does not interpret special characters in paths or other data.
For this specific case, the best fix is to stop using execAsync (based on exec) for the Biome formatting step, and instead invoke yarn (or npx/biome) with an explicit argument list. We can add a new promisified helper built from execFile (e.g., execFileAsync) and use it just for this call. That preserves existing behavior—calling yarn biome check --write --unsafe --config-path=... <shadowPath>—but avoids shell interpolation issues. Concretely in packages/paradigm/scripts/syncFigma.ts:
- Import
execFilein addition toexecfromnode:child_process. - Define
const execFileAsync = promisify(execFile)alongsideexecAsync. - Replace the
execAsync(\yarn biome check ...`)call withexecFileAsync("yarn", ["biome", "check", "--write", "--unsafe", `--config-path=${biomePath}`, shadowPath])`.
No other logic needs to change, and the same Biome formatting is applied to the generated shadows.ts file.
| @@ -1,4 +1,4 @@ | ||
| import { exec } from "node:child_process" | ||
| import { exec, execFile } from "node:child_process" | ||
| import { mkdir, rm, writeFile } from "node:fs/promises" | ||
| import { dirname, join } from "node:path" | ||
| import { fileURLToPath } from "node:url" | ||
| @@ -9,6 +9,7 @@ | ||
| import { letterCase } from "../src/components/Text/letterCase.js" | ||
|
|
||
| const execAsync = promisify(exec) | ||
| const execFileAsync = promisify(execFile) | ||
| const __dirname = dirname(fileURLToPath(import.meta.url)) | ||
|
|
||
| // Load environment variables from .env file | ||
| @@ -375,9 +376,14 @@ | ||
| // Format shadows with Biome | ||
| try { | ||
| const biomePath = join(process.cwd(), "biome.json") | ||
| await execAsync( | ||
| `yarn biome check --write --unsafe --config-path=${biomePath} ${shadowPath}`, | ||
| ) | ||
| await execFileAsync("yarn", [ | ||
| "biome", | ||
| "check", | ||
| "--write", | ||
| "--unsafe", | ||
| `--config-path=${biomePath}`, | ||
| shadowPath, | ||
| ]) | ||
| console.log("✓ Formatted shadows with Biome") | ||
| } catch (_error) { | ||
| console.warn("⚠ Biome formatting failed for shadows, but continuing...") |
| const biomePath = join(process.cwd(), "biome.json") | ||
| const iconPath = join(process.cwd(), "src/components/Icon") | ||
| await execAsync( | ||
| `yarn biome check --write --unsafe --config-path=${biomePath} ${iconPath}`, |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
General fix: Avoid constructing a shell command string with interpolated paths and executing it via a shell (exec). Instead, invoke the command with arguments passed as an array to a non-shell-spawning API such as execFile/spawn, or, since we already use execAsync only here, replace these calls with a wrapper around execFile (or use npx/yarn via arguments). This prevents the shell from interpreting special characters in paths.
Best concrete fix here:
- Stop using
execAsync(based onexec) for the Biome invocations. - Introduce a small helper using
spawnfromnode:child_processthat:- Invokes
yarndirectly with arguments["biome", "check", "--write", "--unsafe", "--config-path", biomePath, targetPath]. - Sets
stdio: "inherit"so output appears normally. - Returns a
Promise<void>that resolves on exit code 0 and rejects otherwise.
- Invokes
- Use this helper in both places where Biome is currently invoked:
- Around line 378–380 for
shadowPath. - Around line 456–460 for
iconPath.
- Around line 378–380 for
- Keep the rest of the logic and error handling intact.
Specifics:
- In
packages/paradigm/scripts/syncFigma.ts:- Replace the
execimport withspawn(no need forpromisify(exec)anymore). - Remove
execAsyncdefinition. - Add a
runBiomehelper function near the top (after env/config setup is fine). - Replace the template-literal
execAsync(...)calls withawait runBiome(biomePath, shadowPath)andawait runBiome(biomePath, iconPath)respectively.
- Replace the
No change in user-visible functionality: we still run the exact same Biome checks on the same paths with the same options, but without passing the full command through a shell.
| @@ -1,4 +1,4 @@ | ||
| import { exec } from "node:child_process" | ||
| import { spawn } from "node:child_process" | ||
| import { mkdir, rm, writeFile } from "node:fs/promises" | ||
| import { dirname, join } from "node:path" | ||
| import { fileURLToPath } from "node:url" | ||
| @@ -8,9 +8,29 @@ | ||
|
|
||
| import { letterCase } from "../src/components/Text/letterCase.js" | ||
|
|
||
| const execAsync = promisify(exec) | ||
| const __dirname = dirname(fileURLToPath(import.meta.url)) | ||
|
|
||
| async function runBiome(configPath: string, targetPath: string): Promise<void> { | ||
| return await new Promise((resolve, reject) => { | ||
| const child = spawn("yarn", ["biome", "check", "--write", "--unsafe", `--config-path=${configPath}`, targetPath], { | ||
| stdio: "inherit", | ||
| shell: false, | ||
| }) | ||
|
|
||
| child.on("error", (error) => { | ||
| reject(error) | ||
| }) | ||
|
|
||
| child.on("exit", (code) => { | ||
| if (code === 0) { | ||
| resolve() | ||
| } else { | ||
| reject(new Error(`Biome exited with code ${code}`)) | ||
| } | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| // Load environment variables from .env file | ||
| config() | ||
|
|
||
| @@ -375,9 +393,7 @@ | ||
| // Format shadows with Biome | ||
| try { | ||
| const biomePath = join(process.cwd(), "biome.json") | ||
| await execAsync( | ||
| `yarn biome check --write --unsafe --config-path=${biomePath} ${shadowPath}`, | ||
| ) | ||
| await runBiome(biomePath, shadowPath) | ||
| console.log("✓ Formatted shadows with Biome") | ||
| } catch (_error) { | ||
| console.warn("⚠ Biome formatting failed for shadows, but continuing...") | ||
| @@ -455,9 +471,7 @@ | ||
| try { | ||
| const biomePath = join(process.cwd(), "biome.json") | ||
| const iconPath = join(process.cwd(), "src/components/Icon") | ||
| await execAsync( | ||
| `yarn biome check --write --unsafe --config-path=${biomePath} ${iconPath}`, | ||
| ) | ||
| await runBiome(biomePath, iconPath) | ||
| console.log("✓ Formatted and organized imports") | ||
| } catch (error) { | ||
| console.warn("⚠ Biome formatting failed, but continuing...") |
There was a problem hiding this comment.
Pull request overview
This PR represents a major architectural shift from Expo/React Native to Vite/React, transforming the client application while enhancing server capabilities and introducing a comprehensive UI component library (Paradigm). The changes modernize the development workflow, improve type safety, and add real-time log streaming capabilities.
Changes:
- Migrated client from Expo to Vite with Monaco editor integration and improved TypeScript support
- Added Paradigm UI component library with Storybook, Tamagui integration, and Figma sync tooling
- Enhanced server with SSE log streaming, improved error handling, and expanded automation schema (draft/icon fields)
Reviewed changes
Copilot reviewed 136 out of 419 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated to version 25.10.1, simplified scripts to use workspace commands, updated Node/Yarn versions |
| apps/client/* | Complete rewrite from Expo to Vite with new store architecture, Monaco editor, and routing via Nuqs |
| apps/server/src/app/services/logger.service.mts | Added SSE log streaming with subscription support and improved filtering |
| apps/server/src/coordinator/services/execute.service.mts | Added formatAutomationContext for consistent logging, improved transpile caching |
| apps/server/src/database/* | Added draft and icon fields to automation schema across all database types |
| apps/server/src/http/services/static.service.mts | Enhanced static file serving with better path resolution and SPA routing |
| packages/paradigm/* | New comprehensive UI component library with Button, Icon components, Storybook setup, and Figma sync |
| Dockerfile | Simplified to single-stage build, improved native module handling, better UTF-8 support |
| .github/workflows/* | Streamlined workflows with dev/main branch builds and concurrency controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // if we have a value we can assume the model has been edited. | ||
| if (currentAutomationRef.current) { | ||
| currentAutomationRef.current.setIsEditied(true) |
There was a problem hiding this comment.
Corrected spelling of 'setIsEditied' to 'setIsEdited'.
| // update the edited state using data and save the draft | ||
| // TODO Move this into the store as a derived state | ||
| if (currentAutomationState) { | ||
| currentAutomationState.setIsEditied( |
There was a problem hiding this comment.
Corrected spelling of 'setIsEditied' to 'setIsEdited'.
| }, | ||
| }) | ||
| .actions({ | ||
| setIsEditied(isEdited: boolean) { |
There was a problem hiding this comment.
Corrected spelling of 'setIsEditied' to 'setIsEdited'.
| setIsEditied(isEdited: boolean) { | |
| setIsEdited(isEdited: boolean) { |
| } catch (err) { | ||
| console.error("[useAutomationLogs] Failed to parse event:", err, event) | ||
| } |
There was a problem hiding this comment.
Using console.error directly in production code. Consider using a proper logging service or error boundary for consistent error handling across the application.
| </head> | ||
| <body> | ||
| <div id="root"></div> | ||
| <script type="module" src="/src/main.jsx"></script> |
There was a problem hiding this comment.
The script points to '/src/main.jsx' but the actual main file is at '/src/main.tsx' (TypeScript). This will cause the application to fail to load.
| <script type="module" src="/src/main.jsx"></script> | |
| <script type="module" src="/src/main.tsx"></script> |
| if (error && typeof error === "object" && "status" in error) { | ||
| logData.code = (error as { status: string | number }).status; | ||
| } |
There was a problem hiding this comment.
The error status could be either string or number, but logData.code type might not accept both. Consider normalizing to a single type (e.g., always convert to number) for consistency.
| if (typeof out === "function") { | ||
| out(); | ||
| } |
There was a problem hiding this comment.
The check for 'typeof out === "function"' seems defensive but the original code assumed 'out' would be called directly. This suggests uncertainty about the return type. Consider adding explicit typing for the 'out' variable to clarify its expected type.
| eventSource.onmessage = (event) => { | ||
| try { | ||
| const data = JSON.parse(event.data) |
There was a problem hiding this comment.
JSON.parse errors are caught but not logged with context about what failed to parse. Add logging to help debug malformed server responses.
No description provided.